Conversation
ab35e4e to
f6ccd12
Compare
|
|
||
| if len(args) > 0 { | ||
| cctx.Logger.Warn("Passing the namespace as an argument is now deprecated; please switch to using -n instead") | ||
| fmt.Fprintln(cctx.Options.Stderr, "Warning: Passing the namespace as an argument is now deprecated; please switch to using -n instead") |
There was a problem hiding this comment.
Why keep the cctx.Logger at all If the cli should not produce structured output?
There was a problem hiding this comment.
Or could you just modify the logger to produce plain text on the correct stream in plain text?
There was a problem hiding this comment.
You're right, it didn't go far enough. I've got rid of the remaining logging, converting one into an unconditional warning. The logger is now only used by the SDK and the server.
|
is there any CI checks that could be put in place to enforce this? |
3d16117 to
5ff9402
Compare
I added a comment to the logger in the struct saying that it is for SDK and server only. Hopefully people won't be tempted to use a logger seeing as it's a CLI. We could also give the logger field in the struct an off-putting name? |
The Fail callback was routing errors through the slog logger, which meant: - Errors appeared as structured log messages (with timestamp and level) - --log-level never silently swallowed errors - --log-format json wrapped errors in JSON Errors are now always printed directly to stderr as "Error: <message>", independent of log configuration. Default log level changed from "info" to "never"; server start-dev continues to default to "warn" via its existing ChangedFromDefault override. Users can still opt in with --log-level on any command. Two deprecation warnings (namespace arg, env command args) converted from logger calls to direct stderr writes so they remain visible regardless of log level.
Convert CLI diagnostic messages (env/config file operations, env-var flag overrides) from structured slog output to plain-text stderr gated on --verbose. The structured logger now exists solely for the SDK client and dev server, which conventionally use it.
--verbose should reveal non-obvious behavior (e.g. env var overriding a flag), not echo what the user just asked to happen.
Set cctx.Verbose before populateFlagsFromEnv so it can call printVerbose immediately, eliminating the deferred callback.
The only printVerbose call site was unreachable (the sole flag with an env var annotation is --env, which is consumed before populateFlagsFromEnv runs). Replace with an unconditional warning to stderr so it works if BindFlagEnvVar is used for other flags in future.
Fixes #567
What was changed
start,listetc, as opposed to the long-runningserver start-devprocess) report errors/warnings as unstructured plain text instead of structured logging-formatted messages.Why?
CLIs should report errors/warnings by printing to stderr. They should not use structured logging messages for this.
How was this tested
A script to manually repro/verify this change is in the git history of this branch. It shows, for example
Scenario: Try to connect to unavailable port
Before
After
Scenario: setting env kv pairs
Before
After
[no output]